Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add scheduled tasks #655

Closed
wants to merge 2 commits into from
Closed

Add scheduled tasks #655

wants to merge 2 commits into from

Conversation

joshbwlng
Copy link
Contributor

@joshbwlng joshbwlng commented Apr 7, 2023 •
edited
Loading

Change-type: minor

[WIP] Add a new tasks vocabulary with job as its primary resource. Consumers can first register new job handlers with addTaskHandler() (similar to addPureHook()), and then schedule tasks for future execution by creating task resources normally. Task executions are cancelled upon their resource counterpart being deleted. Limited retry/backoff logic included on execution failure. During startup, any pending jobs are either executed immediately or scheduled for future execution depending on their start_time date.

Note: Originally started with #654 which uses some existing migration functionality behind the scenes, but decided to try something that was completely decoupled from migrations. I'm looking for something to schedule and execute tasks, not migrations. While we could force the use of migrations, I'm not convinced it's the right decision - don't need async migrations/etc and wouldn't want to pollute migration tables with non-migration task records.

ToDo/Issues:

  • Scope task handlers to models?
  • Allow for retry/backoff value overrides
  • Needs queueing of some sort? Worried about what happens when too many tasks are scheduled to execute at the same time.

Example usage can be found in test/07-tasks.test.ts and test/fixtures/07-tasks/task-handlers.ts.

@joshbwlng joshbwlng force-pushed the joshbwlng/jobs branch 3 times, most recently from 145892e to 61e3df9 Compare April 7, 2023 04:16
@joshbwlng joshbwlng self-assigned this Apr 7, 2023
@fisehara
Copy link
Contributor

@joshbwlng Can we somehow track the actor who caused the job to be scheduled? I think when this is an async data manipulation we still need to track the user for all data changes / transactions that are performed 'later'

@joshbwlng
Copy link
Contributor Author

@fisehara Good point. How about using req.user.id in the create hook, something like:

export const setup = async (tx: Db.Tx) => {
	// Validate and schedule new scheduled jobs for future execution.
	addPureHook('POST', apiRoot, 'scheduled_job', {
		POSTPARSE: async ({ req, request }) => {
			// Set defaults.
			request.values.is_executed_by__user = req.user.id;
			request.values.status = 'pending';
...

@fisehara
Copy link
Contributor

fisehara commented Apr 18, 2023 •
edited
Loading

@joshbwlng to not limit it to a user calling the API we should go and fetch the actorId like we do for the history tables in balena-api:

const getActor = (req: sbvrUtils.HookReq) => {
  const actor = req.user?.actor ?? req.apiKey?.actor;
  if (actor == null) {
	  const err = new Error(
		  `Scheduling job for ${resource} with missing actor on req is not allowed`,
	  );
	  errors.captureException(err, err.message, { req });
	  throw err;
  }
  return actor;
};

The actor will also identify any future usage of API keys that are maybe used for service-to-service integrations that are not strictly bound to a user entity.

@joshbwlng joshbwlng requested a review from fisehara April 19, 2023 01:27
@joshbwlng
Copy link
Contributor Author

@fisehara Nice, thanks, added a new field to the jobs sbvr and some bits to set the actor using basically the example you provided.

@fisehara
Copy link
Contributor

@joshbwlng I have to review it again, but can you please add test cases for the scheduled jobs already? This will demonstrate how they will be used.

@joshbwlng joshbwlng changed the title Add scheduled jobs Add jobs Nov 2, 2023
joshbwlng
joshbwlng commented Nov 2, 2023
View reviewed changes
src/sbvr-api/jobs.ts Outdated Show resolved Hide resolved
@jellyfish-bot
Copy link

[rcooke-warwick] This has attached https://jel.ly.fish/bd214614-d284-46b1-bd7e-1b0b17574605

@joshbwlng joshbwlng force-pushed the joshbwlng/jobs branch 5 times, most recently from 20a4764 to c623c6b Compare November 8, 2023 06:31
@joshbwlng joshbwlng changed the title Add jobs Add tasks Nov 8, 2023
@joshbwlng
Add tasks
b410338 
Change-type: minor
@joshbwlng joshbwlng force-pushed the joshbwlng/jobs branch 3 times, most recently from 2155986 to 6b3bcec Compare November 10, 2023 06:08
@joshbwlng joshbwlng changed the title Add tasks Add scheduled tasks Nov 10, 2023
@joshbwlng joshbwlng force-pushed the joshbwlng/jobs branch 2 times, most recently from 26c1d99 to 667289a Compare November 10, 2023 09:05
@joshbwlng joshbwlng force-pushed the joshbwlng/jobs branch 2 times, most recently from 6739f20 to 44556fb Compare November 15, 2023 08:25
@joshbwlng
Copy link
Contributor Author

Moved to #739

@joshbwlng joshbwlng closed this Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fisehara fisehara Awaiting requested review from fisehara

Assignees

@joshbwlng joshbwlng

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@joshbwlng @fisehara @jellyfish-bot